Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change base image for Dockerfile #2289

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tech-6
Copy link
Contributor

@tech-6 tech-6 commented Nov 24, 2024

  • Default user and group id is now 977 as 999 was taken.

Caution

!!This may cause permissions issues in existing installations.
The above mentioned permissions issues can be fixed by either:

  • mounting the volume with another container as a root user and changing ownership of /app/data (including their contents) to 977:977
  • chmod -R 977:977 /var/lib/docker/volumes/${VOLUME_NAME_OR_HASH} of the volume from the host's volume directory.
  • chmod -R 777 /app/data (don't do this)
  • Dockerfile and entrypoint.sh was changed to run NO application code as a privileged user. When the container is running there is no code executed in a privileged context, as anything requiring privileges is run during build time.
  • Addition of a .dockerignore file to ignore non-application files and improve docker cache hits.

Proposed fix #2288

Default user and group id is now 977 as 999 was taken.
!!**This may cause permissions issues in existing installations**. These issues can be fixed by either:
- mounting the volume with another container as a root user and changing ownership of `/app/data` (including their contents) to 977:977
- `chmod -R 977:977 /var/lib/docker/volumes/${VOLUME_NAME_OR_HASH}` of the volume from the host's volume directory.
- `chmod -R  777 /app/data` (don't do this)

Dockerfile and entrypoint.sh was changed to run NO application code as a privileged user. When the container is running there is no code executed in a privileged context, as anything requiring privileges is run during build time.

Proposed Solution to dzikoysk#2288
@tech-6
Copy link
Contributor Author

tech-6 commented Nov 26, 2024

Seems ready for review, has been stable in my testing environment.

@dzikoysk
Copy link
Owner

Hmm... I'm a bit worried about changing current behaviour - we have quite wide variety of users, even on some exotic setups. That's also the reason why we basically moved a lot of the logic to the entrypoint.sh script.

I think it requires deeper investigation, I'll try to check some historical issues. I guess you could also browse them by searching for Dockerfile or entrypoint keywords.

@dzikoysk dzikoysk added the investigation Issue is currently investigeted, e.g. author is trying to reproduce problem label Nov 27, 2024
@tech-6
Copy link
Contributor Author

tech-6 commented Nov 27, 2024

The main issues that came to mind are #1762, #1200, #1634.

Below is a todo list on testing deployments,

Could be helpful to see if the users from #1657 and #1762 if they got it up and running and for potential testing.

@tech-6
Copy link
Contributor Author

tech-6 commented Nov 27, 2024

Confirmed a working fresh install on RedHat Openshift (see above link)
image
Seems like Openshift's UID randomization still works with user 977 and the rootless container.

@tech-6 tech-6 force-pushed the dockerfile-bellsoft-liberica branch from 8adc663 to 23c5f23 Compare November 27, 2024 13:53
@solonovamax
Copy link
Contributor

A few questions:

  • Why was a liberica jdk chosen over a jdk from eclipse temurin (eg. eclipse-temurin:21-jre-alpine)? What is the benefit of this? Looking at https://whichjdk.com/, there does not seem to be any significant benefit, so I'm unsure why it was chosen.
  • Why was a docker image with Class Data Sharing chosen for the build stage, even though it is never enabled/used?

@dzikoysk
Copy link
Owner

Why was a liberica jdk chosen over a jdk from eclipse temurin (eg. eclipse-temurin:21-jre-alpine)? What is the benefit of this? Looking at https://whichjdk.com/, there does not seem to be any significant benefit, so I'm unsure why it was chosen.

  1. It's smaller
  • eclipse-temurin:21-jre-alpine - 71mb
  • liberica-runtime-container:jre21-slim-musl - 45mb
  • liberica-runtime-container:jre-21-cds-slim-musl - 52mb
  1. I didn't know they have JRE builds

One nice thing about temurin is that maybe we could avoid changing user & group ids.

Why was a docker image with Class Data Sharing chosen for the build stage, even though it is never enabled/used?

I think we should go for the smallest one as long as it works. If that's possible, I guess it'd be nice to have a build without cds.

@tech-6
Copy link
Contributor Author

tech-6 commented Nov 27, 2024

  • Why was a docker image with Class Data Sharing chosen for the build stage, even though it is never enabled/used?

That's on me, Bellsoft's tag page is crazy full, will be pushing no cds on build stage shortly.

@solonovamax
Copy link
Contributor

solonovamax commented Nov 28, 2024

although, using CDS/CRaC features could be something you look into, as it may improve performance (startup time/memory usage).

CRaC would most likely need explicit support from reposilite, javalin, and jetty (jetty already seems to support it, with some additional configuration. see: CRaC/docs/STEP-BY-STEP.md)

but CDS (& AppCDS/Dynamic CDS) could be a free performance boost at startup. (& help with memory? tbh idk exactly how CDS works, but the website I linked said it could help with memory) at the cost of a slightly larger docker image (tbh I wonder if the CDS archive files can be bundled into the jar, so that everyone can benefit from it)

@tech-6
Copy link
Contributor Author

tech-6 commented Nov 28, 2024

I have created an image with static CDS technicolorlabs/reposilite-liberica-testing:dockerfile-bellsoft-liberica-cds dockerfile here, Will definitely take some more looks at app and dynamic CDS some more, with the addition of static CDS it adds approximately 37.5 MB. I don't quite know how to benchmark the two images against each other. But if it offers great enough performance boost it could be worth keeping around.

@dzikoysk
Copy link
Owner

I'd say this is completely out of scope for now to be fair - we should focus on providing smooth migration first.

It'd be great if you could quickly check if using these Temurin images would still require changes in permissions on our side. If no, then maybe we should use it for now. If we'd still need similar changes with Temurin tho, I guess we could proceed with Liberica for Reposilite 3.6.0, since you've already made these changes.

@tech-6
Copy link
Contributor Author

tech-6 commented Nov 29, 2024

If we'd still need similar (uid/gid) changes with Temurin tho, I guess we could proceed with Liberica for Reposilite 3.6.0, since you've already made these changes.

All Alpine based images will have a ping group for group 999.

ping:x:999:

If we change to a ubuntu/debian based image such as eclipse-temurin:21-jre-noble, it would be larger but not have a group 999 conflict.

@dzikoysk
Copy link
Owner

The slim image we're using right now is based on debian, so maybe let's do it the following way:

  1. Move to temurin 21 based on debian in Reposilite 3.5.x
  2. Extend our entrypoint file to provide a migration script that will start using a new default id.
    2.1. If existing data is owned by user with old default id, we'll also automatically try to update it to the new one.
  3. For next minor version (e.g. 3.6.0) we could migrate to liberica with a new setup based on alpine

@tech-6
Copy link
Contributor Author

tech-6 commented Nov 30, 2024

Seems like a pretty good migration strategy, I'll write up changes to the scripts and make a PR, I'll rename this one and maybe return it to draft status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigation Issue is currently investigeted, e.g. author is trying to reproduce problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecation of openJDK docker images
3 participants